refac: Harmonize linopy operations with breaking convention#591
refac: Harmonize linopy operations with breaking convention#591
Conversation
…sets and supersets
Add le(), ge(), eq() methods to LinearExpression and Variable classes, mirroring the pattern of add/sub/mul/div methods. These methods support the join parameter for flexible coordinate alignment when creating constraints.
Consolidate repetitive alignment handling in _add_constant and _apply_constant_op into a single _align_constant method. This eliminates code duplication and makes the alignment behavior (handling join parameter, fill_value, size-aware defaults) testable and maintainable in one place.
numpy_to_dataarray no longer inflates ndim beyond arr.ndim, fixing lower-dim numpy arrays as constraint RHS. Also reject higher-dim constant arrays (numpy/pandas) consistently with DataArray behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use "exact" join for +/- (raises ValueError on mismatch), "inner" join for *// (intersection), and "exact" for constraint DataArray RHS. Named methods (.add(), .sub(), .mul(), .div(), .le(), .ge(), .eq()) accept explicit join= parameter as escape hatch. - Remove shape-dependent "override" heuristic from merge() and _align_constant() - Add join parameter support to to_constraint() for DataArray RHS - Forbid extra dimensions on constraint RHS - Update tests with structured raise-then-recover pattern - Update coordinate-alignment notebook with examples and migration guide Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@FabianHofmann Im quite happy with the notebook now. It showcases the convention and its consequences. |
…ords. Here's what changed: - test_linear_expression_sum / test_linear_expression_sum_with_const: v.loc[:9].add(v.loc[10:], join="override") → v.loc[:9] + v.loc[10:].assign_coords(dim_2=v.loc[:9].coords["dim_2"]) - test_add_join_override → test_add_positional_assign_coords: uses v + disjoint.assign_coords(...) - test_add_constant_join_override → test_add_constant_positional: now uses different coords [5,6,7] + assign_coords to make the test meaningful - test_same_shape_add_join_override → test_same_shape_add_assign_coords: uses + c.to_linexpr().assign_coords(...) - test_add_constant_override_positional → test_add_constant_positional_different_coords: expr + other.assign_coords(...) - test_sub_constant_override → test_sub_constant_positional: expr - other.assign_coords(...) - test_mul_constant_override_positional → test_mul_constant_positional: expr * other.assign_coords(...) - test_div_constant_override_positional → test_div_constant_positional: expr / other.assign_coords(...) - test_variable_mul_override → test_variable_mul_positional: a * other.assign_coords(...) - test_variable_div_override → test_variable_div_positional: a / other.assign_coords(...) - test_add_same_coords_all_joins: removed "override" from loop, added assign_coords variant - test_add_scalar_with_explicit_join → test_add_scalar: simplified to expr + 10
|
The convention should be Why
cost = xr.DataArray([10, 20], coords=[("tech", ["wind", "solar"])])
capacity # dims: (tech=["wind", "solar"], region=["A", "B"])
cost * capacity # ✓ tech matches exactly, region broadcasts freely
capacity.sel(tech=["wind", "solar"]) * renewable_costNo operation should introduce new dimensions Neither side of any arithmetic operation should be allowed to introduce dimensions the other doesn't have. The same problem applies to cost_expr # dims: (tech, time)
regional_expr # dims: (tech, time, region)
cost_expr + regional_expr # ✗ silently expands to (tech, time, region)
capacity # dims: (tech, region, time)
risk # dims: (tech, scenario)
risk * capacity # ✗ silently expands to (tech, region, time, scenario)An explicit pre-check on all operations: asymmetric_dims = set(other.dims).symmetric_difference(set(self.dims))
if asymmetric_dims:
raise ValueError(f"Operation introduces new dimensions: {asymmetric_dims}")Summary
|
Let's clearly differentiate between dimensions and labels. labelsI agree with "exact" for labels by default, but we need an easy way to have inner or outer joining characteristics. I found the pyoframe conventions x + y.keep_extras() to say that an outer join is in order and mismatches should fill with 0. x + y.drop_extras() to say that you want an I have in a different project used | 0 to indicate keep_extras ie (x + y | 0). dimensionsi am actually fond of the ability to auto broadcast over different dimensions. and would want to keep that (actually my main problem with pyoframe). your first example actually implicitly assumes broadcasting. |
Dimensions and broadcastingI agree that auto broadcasting is helpful in some cases. So the full convention requires two separate things: labelsI'm not sure if I like this approach, as it's needs careful state management of the flags on expressions. The flag (keep or drop extras) needs to be handled. import linopy
# outer join — fill gaps with 0 before adding
x_aligned, y_aligned = linopy.align(x, y, join="outer", fill_value=0)
x_aligned + y_aligned
# inner join — drop non-matching coords before adding
x_aligned, y_aligned = linopy.align(x, y, join="inner")
x_aligned + y_alignedCombining disjoint expressions would then still need the explicit methods though. |
|
The proposed convention for all arithmetic operations in linopy: I'm not sure how to implement the | operator yet. Might need some sort of flag/state for defered indexing |
|
I thought about the pipe operator: Would this be an issue for you? |
Spec and tests for commutativity, associativity, distributivity, identity, negation, and zero. Two known violations marked xfail: associativity and distributivity with constants that introduce new dims. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Constants can now introduce new dimensions in arithmetic (+, -, *, /), preserving all standard algebraic laws (associativity, distributivity). The dim-subset check remains for constraint RHS to catch accidental broadcasting. Default fill value for const changed from 0 to NaN. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Constraint RHS can now introduce new dimensions, just like arithmetic. For ==, broadcasting to incompatible values results in solver infeasibility. For <=/>= it creates redundant but harmless constraints. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pure xarray/pandas/numpy operations before entering linopy use their own alignment rules. Document the risks and the xarray exact join workaround. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document assign_coords (recommended) and join="override" for handling operands with mismatched coordinate labels. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coroa @FabianHofmann Im quite happy with this. Looking forward to your thoughts. |
- Remove dead check_common_keys_values function from common.py - Remove as_dataarray from public API exports - Remove redundant aligned_rhs = aligned_rhs in to_constraint - Keep Variable.__mul__ delegating to linexpr (fast path bypasses exact join) - Keep Variable.__div__ delegating to linexpr (error message preserved there) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the comments to two. (I'm writing this the second time as the last post here did not make it.) I agree that the Does that make sense @FBumann @coroa ? About the pipe operator, I am open to that. but it's true that upstream library objects (pandas, xarray) should probably not be monkey-patched (we already do it and I would like to keep it to a minimum). |
|
@FabianHofmann Fully agreed on that. |
|
@FabianHofmann @coroa If you agree i merge #607 in here to get the full diff view and prepare a single PR |
yes, please go ahead |
… as 'no constraint' in RHS - Fill NaN with 0 (add/sub) or fill_value (mul/div) in _add_constant/_apply_constant_op - Fill NaN coefficients with 0 in Variable.to_linexpr - Restore NaN mask in to_constraint() so subset RHS still signals unconstrained positions
Move Self to TYPE_CHECKING block with typing_extensions fallback, since typing.Self is only available in Python 3.11+. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace `join: str | None` with `join: JoinOptions | None` in expressions.py and variables.py to satisfy mypy's xr.align call-overload checking - Add assertion for termination_condition in solvers.py SCIP solver - Add type: ignore comments for xarray merge and reindex edge cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fill NaN values with neutral elements (0 for add, fill_value for mul/div) in constant arithmetic to prevent silent NaN propagation - Fill NaN coefficients in Variable.to_linexpr - Fix conftest.py to use lazy imports avoiding circular import issues Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Capture NaN positions in constant RHS before sub() fills them with 0, then restore NaN afterward so they still signal unconstrained positions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- _align_constant: use override when sizes match, reindex_like otherwise (instead of strict exact join default) - merge: restore check_common_keys_values override/outer logic - to_constraint: restore SUPPORTED_CONSTANT_TYPES conversion with reindex_like and rhs_nan_mask preservation - Sync test_linear_expression.py with origin/harmonize-linopy-operations - Re-add check_common_keys_values to common.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…check Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Add legacy arithmetic join mode with deprecation warning for transition
- Add `options["arithmetic_join"]` setting (default: "legacy") to control
coordinate alignment in arithmetic operations, merge, and constraints
- Legacy mode reproduces old behavior: override when shapes match, outer
otherwise for merge; reindex_like for constants; inner for align()
- All legacy codepaths emit FutureWarning guiding users to opt in to "exact"
- Move shared test fixtures (m, x, y, z, v, u) to conftest.py
- Exact-behavior tests use autouse fixture to set arithmetic_join="exact"
- Legacy test files (test_*_legacy.py) validate old behavior is preserved
- All 2736 tests pass
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Simplify global setting to 'legacy'/'v1', add LinopyDeprecationWarning
- Restrict options["arithmetic_join"] to {"legacy", "v1"} instead of
exposing all xarray join values (explicit join= parameter still accepts any)
- "v1" maps to "exact" join internally
- Add LinopyDeprecationWarning class (subclass of FutureWarning) with
centralized message including how to silence
- Export LinopyDeprecationWarning from linopy.__init__
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Rename arithmetic_join to arithmetic_convention, mention v1 removal
- Rename setting from 'arithmetic_join' to 'arithmetic_convention'
- Update deprecation message: "will be removed in linopy v1"
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Rename test fixtures from exact_join to v1_convention
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Update legacy tests
* Merge harmonize-linopy-operations-mixed, restore NaN filling and align function
- Resolve merge conflicts keeping transition layer logic
- Restore NaN fillna(0) in _add_constant and _apply_constant_op
- Restore simple finisher-based align() function (fixes MultiIndex)
- Use check_common_keys_values in merge legacy path
- Update legacy test files to match origin/harmonize-linopy-operations
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Clean up obsolete code and fix convention-awareness in arithmetic
- Remove dead check_common_keys_values function from common.py
- Remove redundant default_join parameter from _align_constant, use
options["arithmetic_convention"] directly
- Gate fillna(0) calls in _add_constant and _apply_constant_op behind
legacy convention check so NaN values propagate correctly under v1
- Fix legacy to_constraint path to compute constraint RHS directly
instead of routing through sub() which re-applies fillna
- Restore Variable.__mul__ scalar fast path via to_linexpr(other)
- Restore Variable.__div__ explicit TypeError for non-linear division
- Update v1 tests to expect ValueError on mismatched coords and test
explicit join= escape hatches
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix mypy errors and pytest importmode for CI
- Add return type annotations (Generator) to all v1_convention fixtures
- Add importmode = "importlib" to pytest config to fix import mismatch
when linopy is installed from wheel and source dir is also present
- Use tuple literal in loop to fix arg-type error
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix CI: move import linopy to lazy in conftest.py
Top-level `import linopy` in conftest.py caused pytest to import the
package from site-packages before collecting doctests from the source
directory, triggering import file mismatch errors on all platforms.
Move the import inside fixture functions where it's actually needed.
Also revert the unnecessary test.yml and importmode changes.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…erations-mixed # Conflicts: # linopy/expressions.py # linopy/variables.py # test/conftest.py # test/test_constraints.py # test/test_linear_expression.py
- merge() in v1 mode now pre-validates that shared user-dimension coordinates match exactly, then uses outer join for xr.concat (helper dims like _term/_factor are excluded from the check) - Removed redundant pre-checks from LinearExpression.__add__ and QuadraticExpression.__add__ — merge handles enforcement now - Added scalar fast path in _apply_constant_op (mul/div skip alignment) - Wrapped AlignmentError import in try/except for xarray compat - Fixed missing space in __div__ error message - Added .fillna() as escape hatch option 5 in notebook - Updated merge docstring with convention behavior summary - Added explanatory comments (stacklevel, numpy_to_dataarray filtering) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Deduplicate convention-specific test files into single files Merge 4 pairs of v1/legacy test files into single files, eliminating ~2600 lines of duplicated test code. Convention-specific alignment tests are kept in separate classes (V1/Legacy) with autouse fixtures, while shared tests run under the module-level v1 convention. - test_typing_legacy.py -> merged into test_typing.py (parametrized) - test_common_legacy.py -> merged into test_common.py (legacy align test) - test_constraints_legacy.py -> merged into test_constraints.py (legacy alignment class) - test_linear_expression_legacy.py -> merged into test_linear_expression.py (legacy alignment + join classes) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address PR review: consistency, dedup fixtures, missing test - Add legacy_convention fixture to conftest.py; use it consistently instead of manual try/finally blocks (#1) - Parametrize test_constant_with_extra_dims_broadcasts with convention fixture so it runs under both conventions (#2) - Add missing test_quadratic_add_expr_join_inner to TestJoinParameterLegacy (#3) - Extract shared fixtures into _CoordinateAlignmentFixtures and _ConstraintAlignmentFixtures mixin classes to eliminate fixture duplication between V1/Legacy alignment test classes (#4) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
) * Restore master tests, add autouse convention fixture - Restore test files to match master exactly (legacy behavior) - Delete legacy duplicate test files - Add autouse parametrized convention fixture: every test runs under both 'legacy' and 'v1' conventions by default - Add legacy_convention/v1_convention opt-out fixtures for convention-specific tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Mark legacy-only tests, add v1 counterparts for differing behavior Tests that differ between conventions are split: - Legacy-only: marked with legacy_convention fixture (skipped under v1) - V1-only: marked with v1_convention fixture (skipped under legacy) - All other tests: run under both conventions via autouse fixture Files changed: - test_common.py: split test_align into legacy/v1 versions - test_constraints.py: mark TestConstraintCoordinateAlignment as legacy-only, add TestConstraintCoordinateAlignmentV1, split higher-dim RHS tests - test_linear_expression.py: mark TestCoordinateAlignment as legacy-only, add TestCoordinateAlignmentV1, split sum/join tests - test_piecewise_constraints.py: mark legacy-only (implementation not yet v1-compatible) - test_sos_reformulation.py: mark legacy-only (implementation not yet v1-compatible) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Harmonize linopy arithmetic with legacy/v1 convention transition
This PR harmonizes coordinate alignment and NaN handling in linopy arithmetic, with a non-breaking transition layer.
linopy.options["arithmetic_convention"]Two modes:
"legacy"(default) — reproduces current master behavior exactly. EmitsLinopyDeprecationWarningon every legacy codepath."v1"— strict exact-join semantics: mismatched coordinates raiseValueErrorwith helpful messages. NaN values propagate (no implicitfillna).v1 behavior
Exact coordinate matching — Arithmetic operators (
+,-,*,/) require matching coordinates on shared dimensions. Mismatched coordinates raiseValueErrorwith suggestions:Named methods with explicit join —
.add(),.sub(),.mul(),.div(),.le(),.ge(),.eq()acceptjoin=parameter:NaN propagation — NaN in constants propagates through arithmetic (no implicit fillna). Use
.fillna()as an escape hatch:Free broadcasting — Constants can introduce new dimensions. All algebraic laws hold.
merge() behavior
merge()enforces exact matching on shared user-dimension coordinates in v1 mode. Helper dims (_term,_factor) and the concat dim are excluded from this check, since they legitimately differ between expressions. The actualxr.concatusesjoin="outer".In legacy mode, merge uses
overridewhen shared user dims have matching sizes,outerotherwise.Legacy behavior (default, backward-compatible)
Source changes
config.pyLinopyDeprecationWarning,arithmetic_conventionsettingexpressions.pymerge()pre-validates user-dim coords under v1;to_constrainthas separate legacy/v1 paths; scalar fast path for mul/divcommon.pyalign()reads convention (legacy→inner, v1→exact)variables.py__mul__, explicitTypeErrorin__div__,.reindex()methodsmonkey_patch_xarray.pymodel.pyTest structure
test_linear_expression.py,test_constraints.py,test_algebraic_properties.py,test_common.py,test_typing.py): autouse fixture sets v1; test strict matching + explicit join escape hatchestest_*_legacy.py): validate old behavior preserved under"legacy"conftest.py: shared fixtures, lazyimport linopyfor CI compatibilityRollout plan
"legacy"— nothing breakslinopy.options["arithmetic_convention"] = "v1""v1"and drop legacy modeOpen questions
from_tuples/linexpr()— Currently follows the global convention (legacy: override-or-outer, v1: exact). In practice,from_tuplesis always called with same-coord variables (verified across linopy tests and PyPSA), so the convention choice doesn't matter for existing code. Open question: shouldfrom_tuplesaccept ajoin=parameter, or always use a fixed join mode regardless of convention? Modern PyPSA has largely moved to direct arithmetic (cost * var) overlinexpr, so this is low-priority.Test plan
examples/arithmetic-convention.ipynb) with escape hatches including.fillna()🤖 Generated with Claude Code